Fix LabelSelector validation markers for map field #1679

KillianGolds · 2025-10-03T14:06:24Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PR fixes a controller-gen CRD generation error and adds safeguards to prevent similar issues in the future.

Problem:
The MatchLabels field in LabelSelector (api/v1/shared_types.go) had MinItems and MaxItems validation markers applied, but these markers are only valid for array types. Since MatchLabels is defined as map[LabelKey]LabelValue, controller-gen would fail with errors:
must apply minitems to an array
must apply maxitem to an array

Impact:
This caused CRD generation failures for downstream projects (e.g., OpenDataHub KServe) that import Gateway API Inference Extension as a dependency and run controller-gen crd as part of their build process.

Solution:

Commit 1: Fix the validation markers

Changed the validation markers in api/v1/shared_types.go from:

+kubebuilder:validation:MinItems=1 → +kubebuilder:validation:MinProperties=1
+kubebuilder:validation:MaxItems=64 → +kubebuilder:validation:MaxProperties=64

MinProperties and MaxProperties are the correct validation markers for map types and enforce the same constraints (1-64 entries) on the map.

Commit 2: Add CRD generation to `make generate` target

Added controller-gen crd to the generate target to catch invalid kubebuilder markers during development and CI runs.

Previously, make generate only ran controller-gen object, which generates DeepCopy methods but does not validate CRD markers. This meant invalid markers could be merged without detection.

By adding CRD generation to the generate target:

✅ Invalid kubebuilder markers are caught immediately during development
✅ CI will fail if markers are incorrect or CRDs are out of sync (via make verify)
✅ Prevents downstream projects from encountering CRD generation errors
✅ Aligns with the target's documentation which states it generates "CustomResourceDefinition objects"

This change would have caught the MinItems/MaxItems issue before it was merged.

Why wasn't this caught upstream?
The upstream CI only runs make generate, which executes controller-gen object for generating DeepCopy methods. This command does not validate kubebuilder markers or generate CRDs (see controller-gen docs).

The error surfaced downstream in projects that run full CRD generation with controller-gen crd, which validates all kubebuilder markers.

Testing:

✅ CRD generation succeeds with make manifests
✅ Generated CRDs contain correct OpenAPI validation schema (minProperties: 1, maxProperties: 64)
✅ CRD validation passes with kubectl-validate
✅ All unit tests pass

Which issue(s) this PR fixes:
Fixes #1678

Does this PR introduce a user-facing change?:

NONE

k8s-ci-robot · 2025-10-03T14:06:32Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: KillianGolds
Once this PR has been reviewed and has the lgtm label, please assign kfswain for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

netlify · 2025-10-03T14:06:33Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`94e75d0`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/68ed2b744de473000855f001
😎 Deploy Preview	https://deploy-preview-1679--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

linux-foundation-easycla · 2025-10-03T14:06:33Z

The committers listed above are authorized under a signed CLA.

✅ login: KillianGolds / name: Killian Golds (40cecfd, 94e75d0)

k8s-ci-robot · 2025-10-03T14:06:34Z

Welcome @KillianGolds!

It looks like this is your first PR to kubernetes-sigs/gateway-api-inference-extension 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gateway-api-inference-extension has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-10-03T14:06:35Z

Hi @KillianGolds. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

pierDipi · 2025-10-06T13:59:04Z

/cc @nirrozenbaum @kfswain

pierDipi · 2025-10-06T14:00:01Z

/cherry-pick release-1.0

Changed MinItems/MaxItems to MinProperties/MaxProperties for the MatchLabels field in LabelSelector, as it is a map type, not an array. This resolves controller-gen CRD generation errors. Signed-off-by: Killian Golds <[email protected]>

…nifests Gateway API Inference Extension v1.0.0 has invalid kubebuilder validation markers (MinItems/MaxItems on a map field) causing controller-gen to fail. Applied temporary fork workaround pointing to personal fork based on v1.0.0 tag with the fix applied. Changes: - Add replace directive in go.mod for GIE fork at kserve-compatibility-v1.0 - Regenerate all CRD manifests with controller-gen v0.17.2 - Regenerate Go code (deepcopy, openapi) and Python SDK - Fix linting errors in scheduler.go The fork maintains K8s v0.33.4 compatibility (no version conflicts). This is a temporary workaround until upstream merges the fix and releases a patched version. Upstream issue: kubernetes-sigs/gateway-api-inference-extension#1678 Upstream fix PR: kubernetes-sigs/gateway-api-inference-extension#1679 rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED

Add controller-gen crd to the generate target to validate kubebuilder markers during development and CI runs. Previously, make generate only ran controller-gen object, which generates DeepCopy methods but does not validate CRD markers like MinItems, MaxItems, MinProperties, etc. This meant invalid markers could be merged without detection, only to cause failures in downstream projects that run full CRD generation. By adding CRD generation to the generate target: - Invalid kubebuilder markers are caught immediately during development - CI will fail if markers are incorrect or CRDs are out of sync - Prevents downstream projects from encountering CRD generation errors - Aligns with the target's existing documentation which states it generates CustomResourceDefinition objects This change would have caught the MinItems/MaxItems issue fixed in commit 40cecfd before it was merged. Signed-off-by: Killian Golds <[email protected]>

nirrozenbaum · 2025-10-13T20:29:43Z

/ok-to-test

robscott · 2025-10-13T20:35:06Z

Thanks @KillianGolds! This feels like a good candidate for a patch release.

/lgtm
/cc @danehans

k8s-ci-robot · 2025-10-13T20:42:32Z

@KillianGolds: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gateway-api-inference-extension-verify-main	`94e75d0`	link	true	`/test pull-gateway-api-inference-extension-verify-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

nirrozenbaum · 2025-10-13T21:05:24Z

I think you need to regenerate crds for ci to pass. additionally, not sure why we need the changes in makefile.

danehans · 2025-10-13T21:12:00Z

api/v1/shared_types.go

+	// +kubebuilder:validation:MinProperties=1
+	// +kubebuilder:validation:MaxProperties=64


CI is failing because you need to generate the CRDs after making changes to the Go types.

danehans · 2025-10-13T23:21:50Z

@KillianGolds we would like to include this in a patch release. When do you expect to resolve #1679 (comment)?

KillianGolds · 2025-10-13T23:58:30Z

@KillianGolds we would like to include this in a patch release. When do you expect to resolve #1679 (comment)?

Will be first priority tomorrow morning Irish time. Will be resolved within an hour ideally. Just converted to draft untill I address comments and fix crd's but its out of hours for me at the moment so will be done first thing in the morning.

k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. kind/bug Categorizes issue or PR as related to a bug. labels Oct 3, 2025

k8s-ci-robot requested review from liu-cong and robscott October 3, 2025 14:06

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 3, 2025

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 3, 2025

KillianGolds force-pushed the fix-labelselector-validation-markers branch from 893892d to b9b7a12 Compare October 3, 2025 14:12

k8s-ci-robot requested review from kfswain and nirrozenbaum October 6, 2025 13:59

KillianGolds force-pushed the fix-labelselector-validation-markers branch from b9b7a12 to 68f3f39 Compare October 6, 2025 14:45

KillianGolds force-pushed the fix-labelselector-validation-markers branch from 68f3f39 to 40cecfd Compare October 6, 2025 14:49

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 6, 2025

KillianGolds mentioned this pull request Oct 8, 2025

Update Gateway API Inference Extension to v1 opendatahub-io/kserve#914

Draft

5 tasks

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 13, 2025

k8s-ci-robot requested a review from danehans October 13, 2025 20:35

k8s-ci-robot assigned robscott Oct 13, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 13, 2025

danehans reviewed Oct 13, 2025

View reviewed changes

KillianGolds marked this pull request as draft October 13, 2025 21:47

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 13, 2025

		// +kubebuilder:validation:MinProperties=1
		// +kubebuilder:validation:MaxProperties=64

Fix LabelSelector validation markers for map field #1679

Are you sure you want to change the base?

Fix LabelSelector validation markers for map field #1679

Conversation

KillianGolds commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Commit 1: Fix the validation markers

Commit 2: Add CRD generation to make generate target

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

netlify bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for gateway-api-inference-extension ready!

Uh oh!

linux-foundation-easycla bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

k8s-ci-robot commented Oct 3, 2025

Uh oh!

pierDipi commented Oct 6, 2025

Uh oh!

pierDipi commented Oct 6, 2025

Uh oh!

nirrozenbaum commented Oct 13, 2025

Uh oh!

robscott commented Oct 13, 2025

Uh oh!

k8s-ci-robot commented Oct 13, 2025

Uh oh!

nirrozenbaum commented Oct 13, 2025

Uh oh!

danehans Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

danehans commented Oct 13, 2025

Uh oh!

KillianGolds commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

KillianGolds commented Oct 3, 2025 •

edited

Loading

Commit 2: Add CRD generation to `make generate` target

netlify bot commented Oct 3, 2025 •

edited

Loading

linux-foundation-easycla bot commented Oct 3, 2025 •

edited

Loading